-
-
Notifications
You must be signed in to change notification settings - Fork 250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Local/Built-in image proxy #1126
Local/Built-in image proxy #1126
Conversation
…ng variable in hashes.json
I don't believe that this is in scope for spacebar, and it has additional performance considerations. Using Imagor makes more sense to me, as it's a dedicated tool written in a more performant language. |
You don't need to close it if you want to wait for what the others have to say. @Puyodead1 @TheArcaneBrony |
I don't believe this is out of scope for Spacebar? Besides, I don't think performance considerations are that important. For those looking for better performance, Imagor seems to remain available as an option?
|
Yes, it should still be possible to use Imagor and that should be preferred too, but this PR helps if you cannot use Imagor yet. |
honestly idfk, it seems like it could be useful |
And I've been outvoted! @DEVTomatoCake If you can fix the conflicts, I'll merge it |
Currently, there's support for the Imagor image proxy. However, not everyone might be able to run or install it everywhere.
This PR adds a local image proxy which can be configured using the
cdn_imagorServerUrl
config. Currently, it has to be configured as<Base instance URL>/imageproxy
.sharp
(requires C binaries for build)jimp
(pure JS, added as optional dependency)Discussion
(Doesn't have to be on GitHub, Spacebar/Discord is also possible.)
Code
@ts-expect-error
s be fixed in a better way?jimp
is installed, L153 is fine with the@ts-expect-error
comment. If it isn't, TS complains about the directive being unused. How can this be fixed?Concept
cdn_imagorServerUrl
includes/imageproxy
?Config